-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development
: Adjust server tests to restructuring into modules
#9314
Conversation
WalkthroughThe pull request involves the removal of several methods from various repository interfaces in the codebase. These changes affect how certain entities, such as Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approve after new commit
src/test/java/de/tum/cit/aet/artemis/communication/AnswerMessageIntegrationTest.java
Show resolved
Hide resolved
b2b58e7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (6)
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ComplaintResponseRepository.java (0 hunks)
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ResultRepository.java (0 hunks)
- src/main/java/de/tum/cit/aet/artemis/exercise/repository/SubmissionRepository.java (0 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java (0 hunks)
- src/test/java/de/tum/cit/aet/artemis/assessment/TutorEffortIntegrationTest.java (2 hunks)
- src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java (3 hunks)
💤 Files not reviewed due to no reviewable changes (4)
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ComplaintResponseRepository.java
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ResultRepository.java
- src/main/java/de/tum/cit/aet/artemis/exercise/repository/SubmissionRepository.java
- src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java
🧰 Additional context used
📓 Path-based instructions (2)
src/test/java/de/tum/cit/aet/artemis/assessment/TutorEffortIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
📓 Learnings (1)
src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java (1)
Learnt from: Strohgelaender PR: ls1intum/Artemis#8030 File: src/main/java/de/tum/in/www1/artemis/service/CourseScoreCalculationService.java:21-49 Timestamp: 2024-02-16T16:56:16.054Z Learning: The project consistently avoids using star-imports in Java files.
🔇 Additional comments (6)
src/test/java/de/tum/cit/aet/artemis/assessment/TutorEffortIntegrationTest.java (4)
21-22
: LGTM: Import statements updated correctly.The import statements have been updated to use the new test repository classes, which aligns with the PR objective of adjusting server tests to the restructured modules. The naming convention for test repositories is consistent, using the "Test" suffix.
Also applies to: 25-26
60-60
: LGTM: Test case initialization updated correctly.The
initTestCase
method has been updated to use the newTextSubmissionTestRepository
, which aligns with the PR objective of adjusting server tests to the restructured modules. The method name, although long, is descriptive and follows the Spring Data naming conventions.
63-63
: LGTM: User repository updated to test repository.The change from
userRepository
touserTestRepository
is consistent with the overall shift to using test repositories in this integration test. This aligns with the PR objectives and maintains consistency with other test repository usage in the file.
Line range hint
1-143
: Overall assessment: Changes align with PR objectives.The modifications to
TutorEffortIntegrationTest.java
successfully implement the transition to test repositories, which aligns with the PR objective of adjusting server tests to the restructured modules. The existing test logic remains unchanged, maintaining the current test coverage.A minor naming inconsistency was noted for one repository field, but this doesn't impact functionality. Consider addressing this for improved code consistency.
The changes in this file appear to be ready for merging, pending the suggested minor improvement.
src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java (2)
1-1
: Package structure change aligns with modular restructuring.The package change from
de.tum.cit.aet.artemis.service
tode.tum.cit.aet.artemis.assessment.service
appropriately reflects the assessment-specific nature of this service, aligning with the PR's objective of restructuring the code into modules.
52-58
: Variable type updates align with test repository usage.The changes from
StudentParticipationRepository
toStudentParticipationTestRepository
andResultRepository
toResultTestRepository
are consistent with the PR objective of using test repositories in place of production repositories for tests. This separation enhances the distinction between test and production code, promoting better maintainability and reducing potential conflicts.
src/test/java/de/tum/cit/aet/artemis/assessment/TutorEffortIntegrationTest.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM
Checklist
General
Motivation and Context
After restructuring the server (prod) module structure, these changes adjust the test-code structure to the new prod-code structure.
Note: Most of the changes just come from moving files and methods.
Description
In detail, this includes:
test_repository
subpackage to each module if this module uses a test repositoryAnswerMessageIntegrationTest#testCreateAnswerInExamChannel
has been disabled. This test fails withJsonException No _valueDeserializer assigned
but only if run as part of the whole test suite. The root cause for this issue is unclear.Review Progress
Code Review
Summary by CodeRabbit
Bug Fixes
Chores
AuditResource
class into a more specific package for better project structure.